-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Don't write original if it wasn't reprocessed #1993
Don't write original if it wasn't reprocessed #1993
Conversation
@@ -236,6 +236,7 @@ def dirty? | |||
# the instance's errors and returns false, cancelling the save. | |||
def save | |||
flush_deletes unless @options[:keep_old_files] | |||
@queued_for_write.except!(:original) unless @options[:only_process].empty? || @options[:only_process].include?(:original) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line is too long. [127/80]
Does this fix the issue? If so, why? Can you add that context to the commit message? Thank you very much. 👍 |
@@ -236,6 +236,9 @@ def dirty? | |||
# the instance's errors and returns false, cancelling the save. | |||
def save | |||
flush_deletes unless @options[:keep_old_files] | |||
if @options[:only_process].any? && !@options[:only_process].include?(:original) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line is too long. [85/80]
I am not sure that this solution is the best possible, but it fixed the issue for us and we needed to fix it asap. Maybe someone who knows the codebase better than me could offer another way of archiving it. What this code does is it prevents saving of original image if it wasn't reprocessed. When reprocess! method is called only for some styles and original is not included in the list. |
Can you please add a test for this to make sure it doesn't regress in the future? |
before do | ||
rebuild_model( | ||
(aws2_add_region).merge storage: :s3, | ||
styles: { thumb: ["90x90#", :jpg] }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Align the elements of a hash literal if they span more than one line.
it "uploads original" do | ||
@object.expects((defined?(::Aws) ? :upload_file : :write)). | ||
with(anything, content_type: "image/png", | ||
acl: Paperclip::Storage::S3::DEFAULT_PERMISSION).returns(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Align the elements of a hash literal if they span more than one line.
it "uploads original" do | ||
@object.expects((defined?(::Aws) ? :upload_file : :write)). | ||
with(anything, | ||
content_type: "image/png", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Align the parameters of a method call if they span more than one line.
acl: Paperclip::Storage::S3::DEFAULT_PERMISSION).returns(true) | ||
@object.expects((defined?(::Aws) ? :upload_file : :write)). | ||
with(anything, | ||
content_type: "image/jpeg", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Align the parameters of a method call if they span more than one line.
Added tests |
Thanks for your work, @AlexandrZ! Merged into master. 🎉 |
If only_process list is not empty, but it doesn't contain :original style, original file hasn't been reprocessed and it's not needed to rewrite/reupload it.
Fixes #1055 again